Add class parameters, flags, and privateWithin and annotations to newClass in reflect API #21880

jchyb · 2024-11-03T19:29:39Z

Instead of replacing the one newMethod we have, we instead add two more with varying complexity (similarly to how newMethod is handled). This is also so we can keep the initial newClass implementation (the one creating newClass with public empty primary constructor) intact, which despite being experiemental - already sees use in libraries and projects.

Fixes #21739 and addresses some old TODOs (from the stdlibExperimentalDefinitions.scala file).

bishabosha · 2024-11-03T19:54:20Z

library/src/scala/quoted/Quotes.scala

+       *
+       * Parameters can be obtained via classSymbol.memberField
+       */
+      @experimental def newClass(parent: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr], paramNames: List[String], paramTypes: List[TypeRepr], flags: Flags, privateWithin: Symbol): Symbol


would it be possible to reformulate such that parents is also a function (to support param refs?)

Do you mean that it should be a Symbol => List[TypeRepr] type? I have some trouble visualizing where we would need that classSymbol to define the parent TypeRepr (class Cls extends Cls.InnerCls?, is that legal?). Could you give me an example of a tree like that?

Just saw class Cls extends Cls.InnerCls in the wild but in that case, Cls also had a companion object which contained InnerCls - not sure if that is possible to define right now but I'll have to look into the newModule method and see if it also needs updates

I mean something like class anon(x: Int) extends Box[x.type] - I don't know the best solution, but this would be good to be able to support I assume - same with tracked eventually, but maybe that can be cleanly added later?

bishabosha · 2024-11-04T17:44:10Z

tests/neg-macros/newClassParamsMissingArgument/Macro_1.scala

+  import quotes.reflect.*
+
+  val name = nameExpr.valueOrAbort
+  val parents = List(TypeTree.of[Object])


there should be a test case where the parent needs an argument supplied via the constructor parameters

I tried doing that in the not-very-well-named tests/run-macros/newClassParamsExtendsClassParams, which generates something like this:

'{ class `name`(idx: Int) extends Foo(idx) new `name`(22) }

Or do you mean something else?

yeah that looks good

Also such test cases could be added:

Creating a trait with Symbol.newClass (is this possible?)

Creating an abstract class with Symbol.newClass

trait with parameter extending another trait with parameter (not sure about this)

class with multiple parameters (this probably not needed)

extending java class with parameter

jchyb · 2024-11-06T16:33:40Z

To be clear, this is not ready, I did put this up since I wanted some feedback if anyone had some (Thank You @bishabosha!) and did not want to rush it last minute, that's why it a draft

goshacodes · 2025-01-05T08:35:31Z

library/src/scala/quoted/Quotes.scala

+        paramNames: List[String],
+        paramTypes: List[TypeRepr],
+        clsFlags: Flags,
+        clsPrivateWithin: Symbol


Some nit comments.

Since it is public API, maybe align argument names with already existing? E.G. flags and not clsFlags, privateWithin and not clsPrivateWithin

goshacodes · 2025-01-05T08:38:58Z

library/src/scala/quoted/Quotes.scala

+        clsPrivateWithin: Symbol,
+        consFlags: Flags,
+        consPrivateWithin: Symbol,
+        conParamFlags: List[List[Flags]]


constructor related arguments may have same prefix (now there are 3 constructor, cons and con) and maybe put all constructor related arguments together after everything else

goshacodes · 2025-01-05T08:52:41Z

Want this very much 👀

goshacodes · 2025-01-07T10:14:42Z

@jchyb @nicolasstucki Can I somehow help proceeding with this? Maybe contribute with a spree?

jchyb · 2025-01-07T10:33:16Z

It's pretty much done in terms of the difficult stuff. I still need to add some test cases (your suggestions should help, thank you!), docs and perhaps more checks for flags. I planned for this for 3.7, and we still have some time for that, thus the seemingly slow progress.

Gedochao · 2025-01-22T15:08:48Z

This is planned to be included in 3.7.0.

jchyb · 2025-02-17T09:52:22Z

@hamzaremmal @smarter Ideally once we update these methods we'll be able to stabilize them before the next LTS. That is to say, I would really like to merge this before the end of the current cycle, for 3.7. It might seem like a lot of lines of code, but most of those are the added tests and docs - the real changes are all in QuotesImpl.scala (other files like TreeChecker or SourceCode.scala might also have changes to better report errors related to newClass, or better print trees created with newClass).

hamzaremmal

Here is a first part of my review. I'm still looking at QuotesImpl and the tests. I will update the status or submit a new review when I finish.

hamzaremmal · 2025-03-09T14:51:07Z

library/src/scala/quoted/Quotes.scala

@@ -3837,8 +3838,171 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching =>
       *  @note As a macro can only splice code into the point at which it is expanded, all generated symbols must be
       *        direct or indirect children of the reflection context's owner.
       */
-      // TODO: add flags and privateWithin
-      @experimental def newClass(parent: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol
+      @experimental def newClass(owner: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol


Changing the name of a parameter is not something we can do as we wish, since users might be using named parameters in their code and this change can break it. However, this is an experimental method, so we are free to do this change for the time being.

To align with everything else, shouldn't we also change this?

Suggested change

@experimental def newClass(owner: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol

@experimental def newClass(owner: Symbol, name: String, parents: Symbol => List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol

(Not related, but we could extract it into its own issue) I'm not sure how much this change can be useful but it would for sure add expressiveness.

Suggested change

@experimental def newClass(owner: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol

@experimental def newClass(owner: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Symbol => Option[TypeRepr]): Symbol

I wanted to keep that overload of newClass simple and adding this would break current Scalamock implementation (which we should not worry about too much since this method is experimental, but I just did not think it was necessary)

Agreed, as I said, it's not related, but we should consider it at some point.

hamzaremmal · 2025-03-09T15:03:53Z

compiler/src/scala/quoted/runtime/impl/printers/SourceCode.scala

@@ -1379,13 +1379,13 @@ object SourceCode {
        printTypeTree(bounds.low)
      else
        bounds.low match {
-          case Inferred() =>
+          case Inferred() if bounds.low.tpe.typeSymbol == TypeRepr.of[Nothing].typeSymbol =>


This is a cosmetic change and it only hides when we infer Nothing (or Any for the second change). What was the motivation you had behind it?

I had an issue that when I built a tree like class SomeClass[T <: Int] and would call .show on it, it would only render class SomeClass[T]. It turned out that trees created with tpd.TypeTree are treated as Inferred in the Quotes reflect api, and if I wanted to actually render something, I needed this change here (I figure it might be better to have a printout a bit too big, than not big enough). I recall trying to test if it was possible to adjust the reflect Inferred tree definition instead, but I do not remember what the problem there was. Test case where this matters is newClassTypeParams.

hamzaremmal · 2025-03-09T15:09:40Z

library/src/scala/quoted/Quotes.scala

+       *  val clsDef = ClassDef(cls, parents, body = List(fooDef))
+       *  val newCls = Apply(Select(New(TypeIdent(cls)), cls.primaryConstructor), List(idxExpr.asTerm, strExpr.asTerm))
+       *
+       *  Block(List(clsDef), Apply(Select(newCls, cls.methodMember("foo")(0)), Nil)).asExprOf[Unit]


The first overload and the next one have a splice version of the resulting tree in the documentation. This is extracted from the first one:

* constructs the equivalent to * ``` * '{ * class myClass() extends Object with Foo { * def foo(): Unit = println("Calling foo") * } * new myClass(): Foo * } * ```

We should also add that here imho.

hamzaremmal · 2025-03-09T15:23:58Z

compiler/src/dotty/tools/dotc/core/Symbols.scala

+      parentTypes: Symbol => List[Type],
+      selfInfo: Type = NoType,
+      privateWithin: Symbol = NoSymbol,
+      annotations: List[Tree] = Nil,


newNormalizedClassSymbol doesn't actually have the annotations parameter

I added this one myself, we have to set annotations in the denotation, and I thought it would be convenient to do it where we set other denotation related fields

Sure, I was just flagging one of the differences between your new method and newNormalizedClassSymbol . You shouldn't consider it as an actionable item.

hamzaremmal · 2025-03-09T15:27:56Z

compiler/src/dotty/tools/dotc/core/Symbols.scala

+  /** Same as `newNormalizedClassSymbol` except that `parents` can be a function returning a list of arbitrary
+   *  types which get normalized into type refs and parameter bindings.
+   */
+  def newNormalizedClassSymbolUsingClassSymbolinParents(


I'm again not against overloading, but can't we keep the same name as newNormalizedClassSymbol. The proposed one is very (very) long...

Suggested change

def newNormalizedClassSymbolUsingClassSymbolinParents(

def newNormalizedClassSymbol(

hamzaremmal · 2025-03-09T15:31:48Z

compiler/src/dotty/tools/dotc/core/Symbols.scala

+        val parents = parentTypes(cls)
+        assert(parents.nonEmpty && !parents.head.typeSymbol.is(dotc.core.Flags.Trait), "First parent must be a class")
+        denot.info = ClassInfo(owner.thisType, cls, parents.map(_.dealias), decls, TermRef(owner.thisType, module))


You should dealias before checking if the first parent is a trait.

Suggested change

val parents = parentTypes(cls)

assert(parents.nonEmpty && !parents.head.typeSymbol.is(dotc.core.Flags.Trait), "First parent must be a class")

denot.info = ClassInfo(owner.thisType, cls, parents.map(_.dealias), decls, TermRef(owner.thisType, module))

val parents = parentTypes(cls).map(_.dealias)

assert(parents.nonEmpty && !parents.head.typeSymbol.is(dotc.core.Flags.Trait), "First parent must be a class")

denot.info = ClassInfo(owner.thisType, cls, parents, decls, TermRef(owner.thisType, module))

hamzaremmal · 2025-03-09T15:32:45Z

compiler/src/dotty/tools/dotc/core/Symbols.scala

+  /** Same as `newNormalizedModuleSymbol` except that `parents` can be a function returning a list of arbitrary
+   *  types which get normalized into type refs and parameter bindings.
+   */
+  def newNormalizedModuleSymbolUsingClassSymbolInParents(


Same remarks here as for newNormalizedClassSymbolUsingClassSymbolinParents

Suggested change

def newNormalizedModuleSymbolUsingClassSymbolInParents(

def newNormalizedModuleSymbol(

…flect API

bishabosha · 2025-03-12T06:43:24Z

@lihaoyi this should mean you can remove the hacks in Mill for Cross Module macro

…Class in reflect API (#21880) Instead of replacing the one newMethod we have, we instead add two more with varying complexity (similarly to how newMethod is handled). This is also so we can keep the initial newClass implementation (the one creating newClass with public empty primary constructor) intact, which despite being experiemental - already sees use in libraries and projects. Fixes #21739 and addresses some old TODOs (from the stdlibExperimentalDefinitions.scala file).

jchyb added the needs-minor-release This PR cannot be merged until the next minor release label Nov 3, 2024

bishabosha reviewed Nov 3, 2024

View reviewed changes

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from 28cb854 to 33fbd5f Compare November 4, 2024 11:13

bishabosha reviewed Nov 4, 2024

View reviewed changes

tgodzik requested a review from sjrd November 6, 2024 15:51

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from a839263 to 81cea85 Compare January 2, 2025 11:23

goshacodes reviewed Jan 5, 2025

View reviewed changes

goshacodes mentioned this pull request Jan 7, 2025

Allow to mock only traits and abstract classes without parameters ScalaMock/ScalaMock#560

Closed

jchyb changed the title ~~Add class term parameters, flags, and privateWithin to newClass in reflect API~~ Add class parameters, flags, and privateWithin to newClass in reflect API Jan 9, 2025

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from 30abd4f to 85fdd02 Compare January 10, 2025 15:00

jchyb force-pushed the improvement/quotes-new-class-upgrades branch 4 times, most recently from 3ee6271 to c385ec1 Compare February 4, 2025 12:27

jchyb mentioned this pull request Feb 4, 2025

Symbol.newClass does not support class parameters #21739

Closed

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from c385ec1 to 32f484a Compare February 17, 2025 09:34

jchyb changed the title ~~Add class parameters, flags, and privateWithin to newClass in reflect API~~ Add class parameters, flags, and privateWithin to newClass and annotations in reflect API Feb 17, 2025

jchyb requested review from hamzaremmal and smarter and removed request for sjrd February 17, 2025 09:43

jchyb marked this pull request as ready for review February 17, 2025 09:52

jchyb changed the title ~~Add class parameters, flags, and privateWithin to newClass and annotations in reflect API~~ Add class parameters, flags, and privateWithin and annotations to newClass in reflect API Feb 17, 2025

jchyb assigned hamzaremmal Feb 17, 2025

hamzaremmal reviewed Mar 9, 2025

View reviewed changes

jchyb added 9 commits March 10, 2025 14:33

Add class term parameters, flags, and privateWithin to newClass in re…

0f46d4f

…flect API

Add ability to use the class symbol in classdef parents

d6cc6c9

Allow to add type parameters to newClass

5e1eb60

Change parameter names, add tests

ef8853d

Refactor: reuse newClass

9afd40c

Add more tests, checks for flags and improve docs

7e991d2

Allow to annotate classes

0f020f8

Improve api, documentation and add conParamsPrivateWithin

2ca2fc3

Add additional checks for type parameters and update tests

e404de4

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from 8190f1c to 70f3eca Compare March 10, 2025 13:41

address review comments

f4a06bd

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from 70f3eca to f4a06bd Compare March 11, 2025 14:05

hamzaremmal approved these changes Mar 11, 2025

View reviewed changes

hamzaremmal enabled auto-merge (squash) March 11, 2025 15:52

Update positions in a checkfile

aaef412

jchyb force-pushed the improvement/quotes-new-class-upgrades branch from 1575dca to aaef412 Compare March 11, 2025 16:05

hamzaremmal merged commit 9e7aab7 into scala:main Mar 11, 2025
26 of 27 checks passed

hamzaremmal deleted the improvement/quotes-new-class-upgrades branch March 11, 2025 22:31

WojciechMazur added the backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" label Mar 12, 2025

WojciechMazur added this to the 3.7.0 milestone Mar 12, 2025

WojciechMazur added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Mar 12, 2025

tgodzik added the release-notes Should be mentioned in the release notes label Mar 17, 2025

WojciechMazur mentioned this pull request Apr 5, 2025

API derivation broken in Scala 3.7 polyvariant/respectfully#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add class parameters, flags, and privateWithin and annotations to newClass in reflect API #21880

Add class parameters, flags, and privateWithin and annotations to newClass in reflect API #21880

jchyb commented Nov 3, 2024 •

edited

Loading

bishabosha Nov 3, 2024

jchyb Nov 5, 2024

jchyb Nov 5, 2024

bishabosha Nov 5, 2024 •

edited

Loading

bishabosha Nov 4, 2024 •

edited

Loading

jchyb Nov 5, 2024

bishabosha Nov 5, 2024

goshacodes Jan 5, 2025 •

edited

Loading

jchyb commented Nov 6, 2024

goshacodes Jan 5, 2025

goshacodes Jan 5, 2025

goshacodes commented Jan 5, 2025

goshacodes commented Jan 7, 2025

jchyb commented Jan 7, 2025

Gedochao commented Jan 22, 2025

jchyb commented Feb 17, 2025

hamzaremmal left a comment

hamzaremmal Mar 9, 2025

hamzaremmal Mar 9, 2025

hamzaremmal Mar 9, 2025

jchyb Mar 10, 2025

hamzaremmal Mar 10, 2025

hamzaremmal Mar 9, 2025

jchyb Mar 10, 2025 •

edited

Loading

hamzaremmal Mar 9, 2025

hamzaremmal Mar 9, 2025

jchyb Mar 10, 2025

hamzaremmal Mar 10, 2025

hamzaremmal Mar 9, 2025

hamzaremmal Mar 9, 2025

hamzaremmal Mar 9, 2025

bishabosha commented Mar 12, 2025

	@experimental def newClass(owner: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol
	@experimental def newClass(owner: Symbol, name: String, parents: Symbol => List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol

	def newNormalizedClassSymbolUsingClassSymbolinParents(
	def newNormalizedClassSymbol(

	def newNormalizedModuleSymbolUsingClassSymbolInParents(
	def newNormalizedModuleSymbol(

Add class parameters, flags, and privateWithin and annotations to newClass in reflect API #21880

Add class parameters, flags, and privateWithin and annotations to newClass in reflect API #21880

Conversation

jchyb commented Nov 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

bishabosha Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goshacodes Jan 5, 2025 • edited Loading

Choose a reason for hiding this comment

jchyb commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goshacodes commented Jan 5, 2025

goshacodes commented Jan 7, 2025

jchyb commented Jan 7, 2025

Gedochao commented Jan 22, 2025

jchyb commented Feb 17, 2025

hamzaremmal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jchyb Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha commented Mar 12, 2025

jchyb commented Nov 3, 2024 •

edited

Loading

bishabosha Nov 5, 2024 •

edited

Loading

bishabosha Nov 4, 2024 •

edited

Loading

goshacodes Jan 5, 2025 •

edited

Loading

jchyb Mar 10, 2025 •

edited

Loading